Conversation
| @Name(PluginConstants.PROPERTY_NAME_CUSTOM_MESSAGE) | ||
| @Description("Custom message") | ||
| @Nullable | ||
| public String message=PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE; |
There was a problem hiding this comment.
Never set values directly using = in config files, config values are set by platform based on user input on web UI.
| public HelloWorldBatchSourceConfig(){ | ||
| if(message==null || message.isEmpty()) | ||
| { | ||
| message=PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Similar comment here , this can be removed !
| public int getFrequency() { | ||
| return frequency == null ? PluginConstants.PROPERTY_DEFAULT_FREQUENCY : frequency; | ||
| } | ||
| public String getMessage(){ return (message == null || message.isEmpty()) ? PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE: message; } |
| public final class PluginConstants { | ||
| public static final String PLUGIN_NAME = "HelloWorld"; | ||
| public static final String PROPERTY_NAME_FREQUENCY = "frequency"; | ||
| public static final String PROPERTY_NAME_CUSTOM_MESSAGE="Custom Message"; |
There was a problem hiding this comment.
Variable name can be better, this is too generic !
| public static final String PROPERTY_NAME_CUSTOM_MESSAGE="Custom Message"; | ||
| public static final String PROPERTY_CONFIG_FREQUENCY = "cdap.hello.world.config.frequency"; | ||
| public static final int PROPERTY_DEFAULT_FREQUENCY = 1; | ||
| public static final String PROPERTY_CONFIG_DEFAULT_MESSAGE = "Hello world, this is a custom message"; |
There was a problem hiding this comment.
keep the default message same as before for backwards compatibility!
| public HelloWorldInputFormatProvider(HelloWorldBatchSourceConfig config) { | ||
| configMap = new HashMap<>(); | ||
| configMap.put(PluginConstants.PROPERTY_CONFIG_FREQUENCY, Integer.toString(config.getFrequency())); | ||
| configMap.put(PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE, config.getMessage()); |
There was a problem hiding this comment.
This should be a key, although not required it's best to follow the standard, eg : cdap.hello.world.config.frequency
There was a problem hiding this comment.
This one can be cdap.hello.world.config.message
|
Re-base with main and resolve conflicts ! |
|
Resolve merge conflicts and you are missing changes in your |
|
Also lets add a validation the message should not contain |
| @Name(PluginConstants.PROPERTY_NAME_USER_MESSAGE) | ||
| @Description("Custom message") | ||
| @Nullable | ||
| public String message; |
There was a problem hiding this comment.
let's keep these consistent, userMessage is more appropriate.
| configMap.put(PluginConstants.PROPERTY_CONFIG_FREQUENCY, Integer.toString(config.getFrequency())); | ||
|
|
||
| configMap.put(PluginConstants.PROPERTY_CONFIG_KEY_FREQUENCY, Integer.toString(config.getFrequency())); | ||
| configMap.put(PluginConstants.PROPERTY_CONFIG_KEY_MESSAGE, config.getMessage()); |
There was a problem hiding this comment.
PROPERTY_CONFIG_KEY_USER_MESSAGE
2 participants
Added custom message property and made it Nullable to avoid validation error (initialized it to a default message)